Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add visionOS support #1098

Merged

Conversation

stonko1994
Copy link
Contributor

This PR builds upon #1083 but includes CocoaPods and testing support.

Before this PR can be merged, two counterparts PRs in CwlPreconditionTesting need to be merged first:

Based on this PR, I opened a discussion to outline a problematic usage of Carthage.


Checklist - While not every PR needs it, new features should consider this list:

  • Does this have tests?
  • Does this have documentation?
  • Does this break the public API (Requires major version bump)?
  • Is this a new feature (Requires minor version bump)?

@@ -0,0 +1,66 @@
// swift-tools-version:5.9
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was added to not break support for older Swift versions.

See details about this here: https://www.polpiella.dev/version-specific-package-manifests

@stonko1994
Copy link
Contributor Author

@younata I have some questions how to handle certain things in this PR:

  • How to handle the dependency PRs of CwlPreconditionTesting if they are not getting picked up in a timely manner?
  • What is the strategy for the CI and the tests? Currently, it would fail since there is no Xcode 15.1 installed, and therefore, visionOS is unknown. Should we skip the visionOS tests for now and update to the according macOS images once it is included in a stable Xcode version?

@stonko1994 stonko1994 mentioned this pull request Nov 3, 2023
4 tasks
@younata
Copy link
Member

younata commented Nov 3, 2023

This is good work! Thank you so much for pushing for this.

  • How to handle the dependency PRs of CwlPreconditionTesting if they are not getting picked up in a timely manner?

I think what we have here, pointing to a fork, is good for now. I'm hesitant to release a version pointing to the forks, but we'll cross that bridge when we get there. Presumably CwlPreconditionTesting is also hesitant to officially support for an unreleased platform.

  • What is the strategy for the CI and the tests? Currently, it would fail since there is no Xcode 15.1 installed, and therefore, visionOS is unknown. Should we skip the visionOS tests for now and update to the according macOS images once it is included in a stable Xcode version?

I think that's the best we can do for now. We'll probably have to wait even longer for GitHub to add images that include visionOS.

@stonko1994 stonko1994 marked this pull request as ready for review November 4, 2023 10:48
@stonko1994
Copy link
Contributor Author

Thanks 🙇‍♂️ I modified the test script to skip visionOS tests if no SDK is present: b86604f

@stonko1994
Copy link
Contributor Author

@younata, I tried to fix the failing CI steps. Should be working now.

@stonko1994
Copy link
Contributor Author

The PRs in CwlPreconditionTesting were merged and I reverted back to the official repo 💪

@stonko1994 stonko1994 force-pushed the bitmovin/feature/visionos-support branch from 154c37e to 3ff34fc Compare November 14, 2023 11:53
# Conflicts:
#	Carthage/Checkouts/CwlCatchException/Package.swift
#	Carthage/Checkouts/CwlCatchException/README.md
#	Carthage/Checkouts/CwlCatchException/Sources/CwlCatchException/CwlCatchException.swift
#	Carthage/Checkouts/CwlCatchException/Sources/CwlCatchExceptionSupport/CwlCatchException.m
#	Carthage/Checkouts/CwlCatchException/Sources/CwlCatchExceptionSupport/include/CwlCatchException.h
#	Carthage/Checkouts/CwlPreconditionTesting/Package.swift
#	Carthage/Checkouts/CwlPreconditionTesting/Sources/CwlMachBadInstructionHandler/CwlMachBadInstructionHandler.m
#	Carthage/Checkouts/CwlPreconditionTesting/Sources/CwlMachBadInstructionHandler/include/CwlMachBadInstructionHandler.h
#	Carthage/Checkouts/CwlPreconditionTesting/Sources/CwlMachBadInstructionHandler/mach_excServer.c
#	Carthage/Checkouts/CwlPreconditionTesting/Sources/CwlMachBadInstructionHandler/mach_excServer.h
#	Carthage/Checkouts/CwlPreconditionTesting/Sources/CwlPreconditionTesting/CwlBadInstructionException.swift
#	Carthage/Checkouts/CwlPreconditionTesting/Sources/CwlPreconditionTesting/CwlCatchBadInstruction.swift
#	Carthage/Checkouts/CwlPreconditionTesting/Sources/CwlPreconditionTesting/CwlDarwinDefinitions.swift
#	Nimble.podspec
#	Nimble.xcodeproj/project.pbxproj
#	Sources/Nimble/Nimble.h
#	test
@stonko1994
Copy link
Contributor Author

@younata I merged main into this branch to also use the transitive dependency changes 🙌

I would still wait for #1107 to be finished before I update the build on this PR.

@younata
Copy link
Member

younata commented Jan 10, 2024

@stonko1994 I (finally...) got around to getting #1107 working and merged. Looking forward to THAT being taken care of from now on. Please pull in current main, and I'll be happy to merge this as soon as I can.

# Conflicts:
#	Cartfile.private
#	Cartfile.resolved
#	Nimble.xcodeproj/project.pbxproj
Package.resolved Outdated
Copy link
Contributor Author

@stonko1994 stonko1994 Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change to SPM version 2 should be fine, but up for discussion if we want to have it. Version 2 is available since Swift 5.6 with Xcode 13. I did a test opening the project with Xcode 14, and it worked without problems.

@stonko1994
Copy link
Contributor Author

@younata I merged main into this branch 💪 Please execute the CI checks to see if everything works as expected now. Other than that it's ready to be merged from my side.

Copy link
Member

@younata younata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll wait a bit until Github Actions supports Xcode 15.2 before I merge, but this looks pretty good!

@@ -32,7 +32,7 @@ jobs:
runs-on: macos-13
strategy:
matrix:
xcode: ["14.3.1"]
xcode: ["14.3.1", "15.2"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github actions doesn't yet support 15.2 - actions/runner-images#9169

We'll see this test failing until that's merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it is mentioned here 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. Well, excellent. Guess the thread I found was already out of date.

Copy link
Member

@younata younata Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this is annoying. Is the Vision Pro simulator not created by default? https://github.com/Quick/Nimble/actions/runs/7554289300/job/20585517470?pr=1098

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it 😞

6eb5881 I reverted it to unblock this PR. Will keep an eye on the GH runner images and add it once it changes.

Copy link
Contributor Author

@stonko1994 stonko1994 Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just found this issue (which is basically what you linked) where they point out that the version of Xcode 15.2 is the beta 2 and not the final version. I guess it should be fine once they use the stable version of 15.2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll watch that thread and update our CI once a new build is out.

* it looks like the visionOS simulator is not created by default on the used macOS image
@younata younata merged commit 52c7526 into Quick:main Jan 17, 2024
14 of 15 checks passed
cgrindel-self-hosted-renovate bot referenced this pull request in cgrindel/rules_swift_package_manager Jan 18, 2024
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [Quick/Nimble](https://togithub.com/Quick/Nimble) | minor | `from:
"13.1.2"` -> `from: "13.2.0"` |

---

### Release Notes

<details>
<summary>Quick/Nimble (Quick/Nimble)</summary>

### [`v13.2.0`](https://togithub.com/Quick/Nimble/releases/tag/v13.2.0):
- visionOS, map matcher.

[Compare
Source](https://togithub.com/Quick/Nimble/compare/v13.1.2...v13.2.0)

### Highlights

- Nimble now supports visionOS! Thanks
[@&#8203;stonko1994](https://togithub.com/stonko1994)!
- Adds a new `map` matcher. `map` allows you to transform the expression
to another value, and pass that value to another matcher.
- For example, if you wanted to match the first element in a tuple
easily, you could write: `expect(myTuple).to(map(\.0,
equal(expectedValue)))`.
- See [the docs for more
suggestions](https://togithub.com/Quick/Nimble/tree/v13.2.0#mapping-a-value-to-another-value)!

### Autogenerated Release Notes

#### What's Changed

- Add a `map` matcher. by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1112](https://togithub.com/Quick/Nimble/pull/1112)
- Build the carthage frameworks in a github action by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1107](https://togithub.com/Quick/Nimble/pull/1107)
- Bump cocoapods from 1.14.2 to 1.14.3 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/Quick/Nimble/pull/1100](https://togithub.com/Quick/Nimble/pull/1100)
- Add a privacy manifest by
[@&#8203;younata](https://togithub.com/younata) in
[https://github.com/Quick/Nimble/pull/1113](https://togithub.com/Quick/Nimble/pull/1113)
- Add visionOS support by
[@&#8203;stonko1994](https://togithub.com/stonko1994) in
[https://github.com/Quick/Nimble/pull/1098](https://togithub.com/Quick/Nimble/pull/1098)

#### New Contributors

- [@&#8203;stonko1994](https://togithub.com/stonko1994) made their first
contribution in
[https://github.com/Quick/Nimble/pull/1098](https://togithub.com/Quick/Nimble/pull/1098)

**Full Changelog**:
Quick/Nimble@v13.1.2...v13.2.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDAuMCIsInVwZGF0ZWRJblZlciI6IjM2LjEwMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants